Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(dataZoom): valueRange for dataZoom event #17278

Closed
wants to merge 6 commits into from
Closed

feat(dataZoom): valueRange for dataZoom event #17278

wants to merge 6 commits into from

Conversation

susiwen8
Copy link
Contributor

@susiwen8 susiwen8 commented Jun 23, 2022 •
edited by plainheart
Loading

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Expose startValue and endValue at dataZoom event

Fixed issues

Details

Before: What was the problem?

After: How does it behave after the fixing?

Document Info

One of the following should be checked.

  • This PR doesn't relate to document changes
  • The document should be updated later
  • The document changes have been made in apache/echarts-doc#10700

Misc

ZRender Changes

  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

N.A.

Others

Merging options

  • Please squash the commits into a single one when merging.

Other information

@echarts-bot
Copy link

echarts-bot bot commented Jun 23, 2022

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

The pull request is marked to be PR: author is committer because you are a committer of this project.

@susiwen8 susiwen8 requested review from pissang and plainheart June 23, 2022 15:30
@susiwen8 susiwen8 added this to the 5.4 milestone Jun 24, 2022
Ovilia
Ovilia previously approved these changes Jun 24, 2022
View reviewed changes
@echarts-bot
Copy link

echarts-bot bot commented Jun 24, 2022

Document changes are required in this PR. Please also make a PR to apache/echarts-doc for document changes and update the issue id in the PR description. When the doc PR is merged, the maintainers will remove the PR: awaiting doc label.

@Ovilia
Copy link
Contributor

Ovilia commented Jun 24, 2022

The document should be updated by removing "only exists in zoom event of triggered by toolbar".

@Ovilia Ovilia self-requested a review June 24, 2022 02:22
@Ovilia Ovilia added PR: awaiting doc Document changes is required for this PR. and removed PR: doc unchanged labels Jun 24, 2022
@susiwen8 susiwen8 requested a review from Ovilia June 24, 2022 02:57
@susiwen8 susiwen8 mentioned this pull request Jun 24, 2022
Closed
@susiwen8
Copy link
Contributor Author

The document should be updated by removing "only exists in zoom event of triggered by toolbar".

Created a PR for it.

@echarts-bot echarts-bot bot added PR: doc ready and removed PR: awaiting doc Document changes is required for this PR. labels Jun 24, 2022
Ovilia
Ovilia reviewed Jun 28, 2022
View reviewed changes
Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I would like @pissang and @100pah to confirm that the if (dataZoomModel.get('showDetail')) condition was added by mistake.

@pissang
Copy link
Contributor

pissang commented Jun 28, 2022

Looks good. if (dataZoomModel.get('showDetail')) is not a mistake. It's just the old logic only used dataInterval to display the labels and didn't save it to_dataRange.

pissang
pissang previously approved these changes Jun 28, 2022
View reviewed changes
@pissang pissang dismissed their stale review June 28, 2022 03:59

Seems the valueRange is updated after the action is dispatched

@pissang
Copy link
Contributor

pissang commented Jun 28, 2022 •
edited
Loading

Seems the valueRange is updated after the action being dispatched. I guess in the event startValue and endValue will not be the latest value? A test will be good to verify it

@SevenOutman
Copy link

SevenOutman commented Jul 18, 2022 •
edited
Loading

Any update on this PR? This would be really helpful as there's merely no way to map start/end percentage to values in the same way as ECharts does accurately.

@susiwen8
Copy link
Contributor Author

Seems the valueRange is updated after the action being dispatched. I guess in the event startValue and endValue will not be the latest value? A test will be good to verify it

@pissang @Ovilia @plainheart The missing person has returned, Sorry I haven't checked this in time, had very busy two months.

The timing for calculating _valueRange has moved after the calculation of range. This might be the better way, cause dataZoom event rely on range as well as _valueRange, so this two variable could be calculated at same time

@susiwen8 susiwen8 requested review from Ovilia and pissang August 10, 2022 15:10
@Ovilia Ovilia modified the milestones: 5.4, 5.4.1 Sep 1, 2022
@plainheart plainheart mentioned this pull request Nov 15, 2022
Closed
@plainheart plainheart modified the milestones: 5.4.1, 5.5.0 Nov 20, 2022
@ddubrava
Copy link

ddubrava commented Apr 30, 2023 •
edited
Loading

By the way, if an xAxis.type is set to time, a dataZoom.startValue and dataZoom.endValue properties will return formatted values, which makes it difficult to map them back to the original data. For example, if your dataset value is 1682850281010, the dataZoom.startValue will be 1682850281399.8394. Even if you use an ISO string format instead of a timestamp, the dataZoom object will still return a formatted value

@Ovilia Ovilia modified the milestones: 5.5.0, 5.5.1 Jan 18, 2024
@susiwen8 susiwen8 closed this Apr 3, 2024
@plainheart plainheart removed this from the 5.5.1 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plainheart plainheart Awaiting requested review from plainheart

@Ovilia Ovilia Awaiting requested review from Ovilia

@pissang pissang Awaiting requested review from pissang

Assignees
No one assigned
Labels
PR: author is committer PR: doc ready size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add startValue and endValue parameters in zoom event triggered by slider
6 participants
@susiwen8 @Ovilia @pissang @SevenOutman @ddubrava @plainheart